-
Notifications
You must be signed in to change notification settings - Fork 682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix a PIX pass's attempt to set the validator version #6707
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I also don't understand the reasoning, the code and tests look correct. Although I would personally put a blurb on top of each test specifying why each test exists. (Like for example, in validatorNoDowngrade, "This test tests that a shader with dxil validator version of 1.8 does not downgrade to 1.4" and for validatorUpgrade "This test tests that a shader with dxil validator version of 1.3 gets upgraded to 1.4 after the annotate with virtual regs pass.")
Good idea. Your comment suggestions used, almost verbatim |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks fine, though I still don't know why the validator version was upgraded in the first place.
This pass was attempting to compare different things. The return values of GetDxilVersion are not shader models, but... dxil version. Since the code is trying to upgrade the validator version, I changed this to GetValidatorVersion, to pair with SetValidatorVersion.
The previous code was breaking the nvidia driver on workgraphs.